Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Store state when sending commitment_signed #13

Merged
merged 1 commit into from
Apr 16, 2023

Conversation

t-bast
Copy link

@t-bast t-bast commented Mar 28, 2023

If we only store state when sending tx_signatures, there are cases where we cannot reconcile states if a disconnection occurs during the signing steps: one side will have sent tx_signatures and thus must wait for the transaction to be spent or double-spent, while the other side has already forgotten that channel because they haven't sent tx_signatures.

This is fixed by storing state when sending commitment_signed, and adding a next_funding_txid field to channel_reestablish to ask our peer to retransmit signatures that we haven't received.

@t-bast
Copy link
Author

t-bast commented Mar 28, 2023

@niftynei @dunxen @morehouse this commit adds the requirements addressing in this comment. Let me know if that's clear enough, and any feedback you have from implementing it or going through the message flow scenarios described in my comment or in https://gist.github.com/t-bast/1ac31f4e27734a10c5b9847d06db8d86

@dunxen
Copy link

dunxen commented Mar 28, 2023

Thanks for this and for enumerating the disconnection scenarios in the gist! It seems clear IMO, but I will bring it up in LDK's project sync this week and see if there's anything glaringly tricky here. :)

@t-bast t-bast force-pushed the dual-fund-reestablish branch from eccee2d to cc1b3d8 Compare April 3, 2023 07:19
@t-bast
Copy link
Author

t-bast commented Apr 3, 2023

I slightly updated the requirements: the receiver of channel_reestablish doesn't need to retransmit its commit_sig if it has already received the remote tx_signatures (because since the sender has sent tx_signatures, they have stored that commit_sig already), as pointed out by @niftynei

@dunxen
Copy link

dunxen commented Apr 6, 2023

Thanks, LGTM!

02-peer-protocol.md Outdated Show resolved Hide resolved
Copy link

@morehouse morehouse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

If we only store state when sending `tx_signatures`, there are cases where
we cannot reconcile states if a disconnection occurs during the signing
steps: one side will have sent `tx_signatures` and thus must wait for the
transaction to be spent or double-spent, while the other side has already
forgotten that channel because they haven't sent `tx_signatures`.

This is fixed by storing state when sending `commitment_signed`, and
adding a `next_funding_txid` field to `channel_reestablish` to ask our
peer to retransmit signatures that we haven't received.
@t-bast t-bast force-pushed the dual-fund-reestablish branch from b6c4c7c to 248caeb Compare April 13, 2023 12:30
@t-bast
Copy link
Author

t-bast commented Apr 13, 2023

Squashed the fixup commit in 248caeb

Copy link
Owner

@niftynei niftynei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 248caeb

@niftynei niftynei merged commit da9a87e into niftynei:nifty/dual-fund Apr 16, 2023
@t-bast t-bast deleted the dual-fund-reestablish branch June 30, 2023 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants